<html>
<head><meta charset="utf-8"><title>Implement LLVM-compatible source-based cod compiler-team#278 · t-compiler/major changes · Zulip Chat Archive</title></head>
<h2>Stream: <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/index.html">t-compiler/major changes</a></h2>
<h3>Topic: <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html">Implement LLVM-compatible source-based cod compiler-team#278</a></h3>

<hr>

<base href="https://rust-lang.zulipchat.com">

<head><link href="https://rust-lang.github.io/zulip_archive/style.css" rel="stylesheet"></head>

<a name="195224015"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195224015" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> triagebot <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195224015">(Apr 24 2020 at 17:59)</a>:</h4>
<p>A new proposal has been announced <a href="https://github.com/rust-lang/compiler-team/issues/278" title="https://github.com/rust-lang/compiler-team/issues/278">#278</a>. It will be brought up at the next meeting.</p>



<a name="195842947"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195842947" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Wesley Wiser <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195842947">(Apr 30 2020 at 14:02)</a>:</h4>
<p>I do have a few questions from a detailed reading of your proposal, the PR and the related issues:</p>



<a name="195842974"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195842974" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Wesley Wiser <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195842974">(Apr 30 2020 at 14:02)</a>:</h4>
<ul>
<li>Since the llvm-profdata tool is tied to the LLVM version, how do we expect users to obtain that tool? Will they be required to build our LLVM fork or will the tool be available via rustup? I think, as this is an unstable compiler flag, pretty much any answer is fine but we should have some plan for stabilizing this flag.</li>
</ul>



<a name="195842995"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195842995" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Wesley Wiser <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195842995">(Apr 30 2020 at 14:03)</a>:</h4>
<ul>
<li>Introducing so many calls to the __incr_cov function will create a lot of additional control flow edges (and a lot more MIR). It seems like this could lead to a lot of additional work done during compile time. Does clang take a similar approach? What impact does that typically have on their compilation time? How is the compilation performance of the existing prototype you have? Do you have any thoughts about whether this hits an "acceptable" bar for compiler performance?</li>
</ul>



<a name="195843016"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195843016" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Wesley Wiser <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195843016">(Apr 30 2020 at 14:03)</a>:</h4>
<ul>
<li>Like many others on the PR and linked issues, my initial thought was that this would be a MIR pass. Having used poor quality tools in the past, I certainly agree with @bob-wilson's points regarding the quality of the code coverage reporting being critical to this feature. Still, my gut says that this should be done at least at the HIR level. I think I'm worried that by basing this pass on the AST, it will break much more frequently as the AST is changed than if it was based on a lower-level construct like the HIR. Do you have thoughts about what could be done to minimize regressions in the code coverage due to changes in the AST, for example, when adding new syntax?</li>
</ul>



<a name="195843028"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195843028" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Wesley Wiser <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195843028">(Apr 30 2020 at 14:03)</a>:</h4>
<ul>
<li>It sounds like you implemented two prototypes, one based on MIR transforms and one on the AST approach. Did you find issues regarding the quality of code coverage information generated from the MIR pass that the AST approach fixed? Or was the primary issue the implementation complexity that you mentioned above?</li>
</ul>



<a name="195845167"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195845167" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195845167">(Apr 30 2020 at 14:17)</a>:</h4>
<p><span class="user-mention silent" data-user-id="125250">Wesley Wiser</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195842974" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195842974">said</a>:</p>
<blockquote>
<ul>
<li>Since the llvm-profdata tool is tied to the LLVM version, how do we expect users to obtain that tool? Will they be required to build our LLVM fork or will the tool be available via rustup? I think, as this is an unstable compiler flag, pretty much any answer is fine but we should have some plan for stabilizing this flag.</li>
</ul>
</blockquote>
<p>I think it's sufficient, at least initially, to assume the first significant users will integrate the llvm-profdata and llvm-cov tools into their own toolchains. This is what the Fuchsia team will be doing once coverage is available. Downloading and/or compiling the additional tools, and scripting commands with the project-appropriate flags gives each project owner the most flexibility.</p>
<p>For any end-user that is savvy enough to want to integrate coverage analysis into even small projects, the <a href="https://clang.llvm.org/docs/SourceBasedCodeCoverage.html" title="https://clang.llvm.org/docs/SourceBasedCodeCoverage.html">clang example tutorial</a> I referenced in the MCP is fairly straightforward to run, and is easily scriptable.</p>
<p>I can also imagine future efforts, if someone is motivated to do so, to integrated the Rust coverage features into IDEs (VSCode, etc.) and CI tools (Travis, etc.).</p>



<a name="195850784"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195850784" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195850784">(Apr 30 2020 at 14:55)</a>:</h4>
<p><span class="user-mention silent" data-user-id="125250">Wesley Wiser</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195842995" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195842995">said</a>:</p>
<blockquote>
<ul>
<li>Introducing so many calls to the __incr_cov function will create a lot of additional control flow edges (and a lot more MIR). It seems like this could lead to a lot of additional work done during compile time. Does clang take a similar approach? What impact does that typically have on their compilation time? How is the compilation performance of the existing prototype you have? Do you have any thoughts about whether this hits an "acceptable" bar for compiler performance?</li>
</ul>
</blockquote>
<p>It's too early to tell what the performance impact is or what kind of optimizations can be done. (This touches a little on your third bullet as well, so I'll say more on that point in a separate response.)</p>
<p>My prototyping, thus far, is focused on injecting coverage counters, computing the associated coverage regions, and running the resulting program to confirm the results match expectations, so I've done feasibility prototyping, but nothing significant enough to assess performance, yet. I certainly care about performance, and I continue to look into low-hanging-fruit opportunities where available. But other optimizations will have to be done in phases, as mentioned in the MCP, once the base capability is known to work.</p>
<p>Also, since coverage injection will almost certainly be opt-in, and only done very occasionally, the bar for "acceptable" performance is not going to be nearly the same as compiling without that instrumentation. I do expect the impact to be linear to the number of existing branches in the source.</p>
<p><span class="user-mention silent" data-user-id="296362">Bob Wilson</span> can speak more authoritatively to the clang approach (ref: <a href="https://clang.llvm.org/doxygen/CoverageMappingGen_8cpp_source.html" title="https://clang.llvm.org/doxygen/CoverageMappingGen_8cpp_source.html">https://clang.llvm.org/doxygen/CoverageMappingGen_8cpp_source.html</a>) but my take is that clang is processing the AST and injecting the counters from there. I don't think clang has a direct analogy to the Rust HIR representation, but the clang coverage pass might be considered as happening at a level similar to the AST-&gt;HIR lowering pass in <code>rustc_ast_lowering</code>. Both walk the AST in a similar way.</p>



<a name="195854114"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195854114" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> tmandry <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195854114">(Apr 30 2020 at 15:14)</a>:</h4>
<p>A problem with drawing an analogy to clang is that clang's AST isn't really an AST (it has a lot more HIR-like information hanging off it)</p>



<a name="195854263"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195854263" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> tmandry <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195854263">(Apr 30 2020 at 15:15)</a>:</h4>
<p>clang also has no MIR equivalent that I know of</p>



<a name="195857262"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195857262" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Wesley Wiser <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195857262">(Apr 30 2020 at 15:35)</a>:</h4>
<p><span class="user-mention silent" data-user-id="296355">Rich Kadel</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195845167" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195845167">said</a>:</p>
<blockquote>
<p><span class="user-mention silent" data-user-id="125250">Wesley Wiser</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195842974" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195842974">said</a>:</p>
<blockquote>
<ul>
<li>Since the llvm-profdata tool is tied to the LLVM version, how do we expect users to obtain that tool? Will they be required to build our LLVM fork or will the tool be available via rustup? I think, as this is an unstable compiler flag, pretty much any answer is fine but we should have some plan for stabilizing this flag.</li>
</ul>
</blockquote>
<p>I think it's sufficient, at least initially, to assume the first significant users will integrate the llvm-profdata and llvm-cov tools into their own toolchains. This is what the Fuchsia team will be doing once coverage is available. Downloading and/or compiling the additional tools, and scripting commands with the project-appropriate flags gives each project owner the most flexibility.</p>
<p>For any end-user that is savvy enough to want to integrate coverage analysis into even small projects, the <a href="https://clang.llvm.org/docs/SourceBasedCodeCoverage.html" title="https://clang.llvm.org/docs/SourceBasedCodeCoverage.html">clang example tutorial</a> I referenced in the MCP is fairly straightforward to run, and is easily scriptable.</p>
<p>I can also imagine future efforts, if someone is motivated to do so, to integrated the Rust coverage features into IDEs (VSCode, etc.) and CI tools (Travis, etc.).</p>
</blockquote>
<p>Sounds great! Thanks</p>



<a name="195858079"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195858079" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Wesley Wiser <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195858079">(Apr 30 2020 at 15:40)</a>:</h4>
<blockquote>
<p>Compiler performance impact</p>
</blockquote>
<p>That makes sense. I agree the threshold is probably significantly higher given that this opt-in for people who want the feature and it should have absolutely no impact otherwise. </p>
<p>I would caution about optimizations on AST/HIR in the future, AFAIK, we have none of those currently or any plans to implement them. Doing any optimizations in <code>rustc</code> was pretty heavily gated on the MIR effort because it just wasn't practical before that point.</p>



<a name="195858195"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195858195" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195858195">(Apr 30 2020 at 15:41)</a>:</h4>
<p><span class="user-mention silent" data-user-id="125250">Wesley Wiser</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195843016" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195843016">said</a>:</p>
<blockquote>
<ul>
<li>Like many others on the PR and linked issues, my initial thought was that this would be a MIR pass. Having used poor quality tools in the past, I certainly agree with @bob-wilson's points regarding the quality of the code coverage reporting being critical to this feature. Still, my gut says that this should be done at least at the HIR level. I think I'm worried that by basing this pass on the AST, it will break much more frequently as the AST is changed than if it was based on a lower-level construct like the HIR. Do you have thoughts about what could be done to minimize regressions in the code coverage due to changes in the AST, for example, when adding new syntax?</li>
</ul>
</blockquote>
<p>Yes! I discussed three alternatives in the MCP, and I'm open to influence here. You make a good point about the stability of the AST vs. HIR, an I'm open to influence here. I do see value in an HIR-level approach.</p>
<p>My chief concern is that I need to understand the representation well enough to accurately identify the coverage regions within the original source code, and accurately inject the right counter instructions in the right places. At least with the AST, it's obvious. </p>
<p>I'm already investigating how I can do this within the HIR, and I'd like to do it there if I can ramp up quickly enough, but I also have working code built on the AST. It may or may not be a reasonable cost-benefit tradeoff, for the <strong>initial</strong> implementation, but if not, it shouldn't be hard to move the coverage-injected nodes to the HIR in a follow-on optimization pass. (The question, then, would be, once there's a proven implementation, would someone then say they can bypass the HIR altogether and just migrate coverage injection directly to a MIR pass? And if that's the case, then spending time trying to move working AST code to HIR might be a waste of time anyway ;-) ...</p>
<p>All this said, I'm not ignoring the feedback here and I am investigating those alternatives.</p>



<a name="195858644"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195858644" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Wesley Wiser <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195858644">(Apr 30 2020 at 15:44)</a>:</h4>
<p>Yeah, I didn't mean to imply that you were ignoring the feedback. The entire value proposition of this feature is that it closely matches the end-user's mental model of the language semantics, not an underlying thing like MIR. So I fully agree with you and <span class="user-mention" data-user-id="296362">@Bob Wilson</span> there!</p>



<a name="195858673"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195858673" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195858673">(Apr 30 2020 at 15:44)</a>:</h4>
<p><span class="user-mention silent" data-user-id="125250">Wesley Wiser</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195858079" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195858079">said</a>:</p>
<blockquote>
<p>I would caution about optimizations on AST/HIR in the future, AFAIK, we have none of those currently or any plans to implement them. Doing any optimizations in <code>rustc</code> was pretty heavily gated on the MIR effort because it just wasn't practical before that point.</p>
</blockquote>
<p>Understood. I think my use of the word optimization here refers only to optimizing the implementation of coverage injection, for example, moving coverage injection from earlier passes to later passes, or even simple things like improving how the hash argument is computed.</p>



<a name="195858676"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195858676" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Wesley Wiser <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195858676">(Apr 30 2020 at 15:44)</a>:</h4>
<p>I'm just wanting to try to find a way to have our cake and eat it too. <span aria-label="slight smile" class="emoji emoji-1f642" role="img" title="slight smile">:slight_smile:</span></p>



<a name="195858998"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195858998" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> simulacrum <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195858998">(Apr 30 2020 at 15:47)</a>:</h4>
<p>one question I also had -- to what extent do we expect this to be stable? I presume if e.g. rustc 1.50 decided to refactor how things internally were represented and that meant that coverage for some things vanished (e.g. end brace of a for loop or some such) people wouldn't be too concerned?</p>



<a name="195859680"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195859680" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> tmandry <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195859680">(Apr 30 2020 at 15:51)</a>:</h4>
<p>The effect of that would be degraded behavior, but not breaking anything per se</p>



<a name="195859785"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195859785" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> tmandry <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195859785">(Apr 30 2020 at 15:52)</a>:</h4>
<p>That's one argument I can see in favor of doing it in a later stage like MIR; fewer edge cases to worry about or that can break.</p>



<a name="195860160"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195860160" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> tmandry <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195860160">(Apr 30 2020 at 15:54)</a>:</h4>
<p>Another argument is that it might make it easier to use performance counter info directly in MIR optimizations one day, though I'm not 100% clear on exactly how that would work.</p>



<a name="195860554"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195860554" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> simulacrum <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195860554">(Apr 30 2020 at 15:57)</a>:</h4>
<p>we could e.g. annotate likely/unlikely branches based on perf counters ourselves, though it's probably better to leave that to llvm I imagine</p>



<a name="195860622"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195860622" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> simulacrum <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195860622">(Apr 30 2020 at 15:57)</a>:</h4>
<p>maybe with e.g. cranelift which doesn't have native support for pgo though that would make more sense</p>



<a name="195861306"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195861306" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195861306">(Apr 30 2020 at 16:01)</a>:</h4>
<p><span class="user-mention silent" data-user-id="125250">Wesley Wiser</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195843028" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195843028">said</a>:</p>
<blockquote>
<ul>
<li>It sounds like you implemented two prototypes, one based on MIR transforms and one on the AST approach. Did you find issues regarding the quality of code coverage information generated from the MIR pass that the AST approach fixed? Or was the primary issue the implementation complexity that you mentioned above?</li>
</ul>
</blockquote>
<p>To be more clear, I did start an early prototype by injecting coverage in <code>rustc_ast_lowering</code> functions, but as I started working out how to ensure proper coverage for the less simple cases (like lazy booleans, for example), I felt I had more control and visibility by implementing coverage by walking the AST before any HIR code transformations. (Still considering the HIR though, as discussed in my earlier reply.)</p>
<p>I did not code up any prototype on the MIR <em>directly</em> but I used -Zunpretty=mir to get a look at the difference between the uninstrumented MIR and an instrumented MIR (after I had injected counter code), and from that I decided the MIR might be a bridge too far, for me, from a complexity standpoint.</p>



<a name="195862364"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195862364" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Amanieu <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195862364">(Apr 30 2020 at 16:08)</a>:</h4>
<p>I recently implemented code coverage support for embedded Rust based on the existing -Zprofile GCOV profiling support: <a href="https://github.com/Amanieu/minicov/" title="https://github.com/Amanieu/minicov/">https://github.com/Amanieu/minicov/</a></p>
<p>It basically works by using a custom implementation of the profiling runtime which simply serializes the counters and metadata to a <code>Vec&lt;u8&gt;</code>. This data can then be sent back to the dev system and "replayed", which creates the .gcda files as if the program was run locally.</p>
<p>Would such an approach also be possible for new profiling system considering the constraints of embedded systems (<code>#[no_std]</code> and no libc but you can use liballoc)? It seems to me that the new profiling system would produce much more accurate profiling information than the existing GCOV-based system which is injected in an LLVM pass.</p>



<a name="195862740"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195862740" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> tmandry <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195862740">(Apr 30 2020 at 16:11)</a>:</h4>
<p><span class="user-mention" data-user-id="143274">@Amanieu</span> Possibly, but the actual implementation of maintaining the counters is all in LLVM</p>



<a name="195864923"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195864923" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195864923">(Apr 30 2020 at 16:27)</a>:</h4>
<p>On a related note (to the discussion with Wesley on AST vs HIR), I'd like to look into the possible benefits of mirroring what is done in <a href="https://github.com/rust-lang/rust/blob/master/src/librustc_ast_lowering/expr.rs#L1300" title="https://github.com/rust-lang/rust/blob/master/src/librustc_ast_lowering/expr.rs#L1300"><code>expr_drop_temps...()</code> in <code>rustc_ast_lowering/expr.rs</code></a>. It appears to inject a custom HIR node <code>hir::ExprKind::DropTemps(expr)</code> that (according to the docs):</p>
<div class="codehilite"><pre><span></span><code>    /// ... has the same effect as wrapping `expr` in
    /// `{ let _t = $expr; _t }` but should provide better compile-time performance.
</code></pre></div>


<p>This is very reminiscent of the unwrapped implementation of my notional inlined <code>__incr_cov()</code> implementation. (Aside: I am not comfortable that the inlining will work as expected so my plan is to implement the functionality unwrapped, without introducing an intermediate function call.) So assuming the following <code>match</code> is part of a coverage region that needs a counter:</p>
<div class="codehilite"><pre><span></span><code>    match some_expr { ...
</code></pre></div>


<p>Injecting the LLVM intrinsic counter function would look something like:</p>
<div class="codehilite"><pre><span></span><code>    match { let _t = some_expr; llvm_instrprof_increment(...); _t } {
</code></pre></div>


<p>I'm interested to get feedback from someone familiar with the DropTemps implementation and ultimate code injection if a similar strategy can be used for the coverage counter injection, and if that would also give us some "better compile time performance".</p>



<a name="195865407"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195865407" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> tmandry <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195865407">(Apr 30 2020 at 16:31)</a>:</h4>
<p><span class="user-mention silent" data-user-id="296355">Rich Kadel</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195861306" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195861306">said</a>:</p>
<blockquote>
<p>I did not code up any prototype on the MIR <em>directly</em> but I used -Zunpretty=mir to get a look at the difference between the uninstrumented MIR and an instrumented MIR (after I had injected counter code), and from that I decided the MIR might be a bridge too far, for me, from a complexity standpoint.</p>
</blockquote>
<p>So a benefit of MIR is that every possible branch is encoded directly in the representation (it's a basic block), and that includes things not obvious from the AST, like panic/unwind branches. The question in my mind is whether there's a tradeoff between instrumenting this "invisible code" and having full fidelity on covering the source code. I think full fidelity on the source is <em>most</em> important, but having coverage on all generated code is important too for PGO.</p>
<p>MIR's SourceInfo maps every statement and terminator back to the source code from which it came, and directly after HIR -&gt; MIR construction I would expect that to be quite high fidelity. <span class="user-mention" data-user-id="119009">@eddyb</span> also seems to think this is the way to go.</p>



<a name="195865588"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195865588" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195865588">(Apr 30 2020 at 16:32)</a>:</h4>
<p><span class="user-mention" data-user-id="296355">@Rich Kadel</span> so the decision was based on what you would've had to add in MIR, i.e. the calls? not anything about what information MIR had for you to be able to decide <em>where</em> to instrument?</p>



<a name="195865622"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195865622" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195865622">(Apr 30 2020 at 16:33)</a>:</h4>
<p>because this is something that confused me</p>



<a name="195865676"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195865676" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195865676">(Apr 30 2020 at 16:33)</a>:</h4>
<p><span class="user-mention" data-user-id="296355">@Rich Kadel</span> anything you inject early-on will produce messy MIR - that doesn't mean that doing it at the MIR level would require all of that mess</p>



<a name="195865703"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195865703" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195865703">(Apr 30 2020 at 16:33)</a>:</h4>
<p>in fact, it's probably much better to try to avoid it in the first place</p>



<a name="195865741"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195865741" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195865741">(Apr 30 2020 at 16:34)</a>:</h4>
<p>for example, if you introduce a new MIR statement, you can avoid calls altogether</p>



<a name="195865792"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195865792" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> tmandry <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195865792">(Apr 30 2020 at 16:34)</a>:</h4>
<p>I think <span class="user-mention" data-user-id="296362">@Bob Wilson</span> (hopefully that's the right person) had looked into this before and decided not to do it in MIR, but I'm not sure what their reasoning was.</p>



<a name="195865809"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195865809" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195865809">(Apr 30 2020 at 16:34)</a>:</h4>
<p>MIR calls are terminators because they may unwind</p>



<a name="195865855"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195865855" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195865855">(Apr 30 2020 at 16:34)</a>:</h4>
<p><span class="user-mention" data-user-id="116883">@tmandry</span> oh okay. still, this confusion/question applies to anyone who doesn't want to do something on/with/etc. MIR</p>



<a name="195865864"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195865864" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195865864">(Apr 30 2020 at 16:34)</a>:</h4>
<p><span class="user-mention silent" data-user-id="119009">eddyb</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195865676" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195865676">said</a>:</p>
<blockquote>
<p><span class="user-mention silent" data-user-id="296355">Rich Kadel</span> anything you inject early-on will produce messy MIR - that doesn't mean that doing it at the MIR level would require all of that mess</p>
</blockquote>
<p>Yes, I do understand that.</p>



<a name="195866120"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195866120" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195866120">(Apr 30 2020 at 16:36)</a>:</h4>
<p>okay reading more of the backlog, <code>DropTemps</code> is basically a noop for almost everything except one part of the compiler</p>



<a name="195866198"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195866198" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195866198">(Apr 30 2020 at 16:37)</a>:</h4>
<p>But I look at the MIR and see a bunch of new <code>let</code> statements with auto generated names, etc. etc. and it's just hard to trace back to how to ensure that generating code at that level will give me the result I expect when looking at the source. It's mostly my lack of being able to grok the MIR.</p>



<a name="195866383"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195866383" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195866383">(Apr 30 2020 at 16:38)</a>:</h4>
<p>if you want to go that route (<code>DropTemps</code>-like) you could add a "post-instrument" expression, that evaluates the original expression and then immediately afterwards does the instrumentation</p>



<a name="195866478"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195866478" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> tmandry <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195866478">(Apr 30 2020 at 16:39)</a>:</h4>
<p>/me dreams about a vscode extension or something that lets you mouse over a MIR statement and see the SourceInfo associated with it as a highlight in your editor</p>



<a name="195866533"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195866533" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195866533">(Apr 30 2020 at 16:39)</a>:</h4>
<p><span class="user-mention" data-user-id="296355">@Rich Kadel</span> okay but shouldn't that (groking MIR) be a necessary step before you decide how to implement this?</p>



<a name="195866787"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195866787" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195866787">(Apr 30 2020 at 16:41)</a>:</h4>
<p>it's also unclear to me, from the <code>match</code> example, what it is achieved, since there's no way to record which arm matched</p>



<a name="195866800"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195866800" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195866800">(Apr 30 2020 at 16:41)</a>:</h4>
<p>I don't see that as a significant requirement. (I agree I have to understand how it works at some level, but not necessarily how it remaps the control flow.) A Rust programmer doesn't have to grok MIR to write Rust.</p>



<a name="195866847"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195866847" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195866847">(Apr 30 2020 at 16:41)</a>:</h4>
<p>I mean for implementing this feature, not using it</p>



<a name="195867008"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195867008" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195867008">(Apr 30 2020 at 16:42)</a>:</h4>
<p><span class="user-mention silent" data-user-id="119009">eddyb</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195866787" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195866787">said</a>:</p>
<blockquote>
<p>it's also unclear to me, from the <code>match</code> example, what it is achieved, since there's no way to record which arm matched</p>
</blockquote>
<p>The example is not counting the arms. Its counting the code leading up to the execution of the pattern match, including any code inside the match expression (with the possibility that an insanely complex match expression could include an early return).</p>



<a name="195867045"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195867045" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> tmandry <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195867045">(Apr 30 2020 at 16:43)</a>:</h4>
<p><span class="user-mention silent" data-user-id="119009">eddyb</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195866533" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195866533">said</a>:</p>
<blockquote>
<p><span class="user-mention silent" data-user-id="296355">Rich Kadel</span> okay but shouldn't that (groking MIR) be a necessary step before you decide how to implement this?</p>
</blockquote>
<p>Meta-point: I mean, that's part of what a design review is for. Taking a stab at a design based on limited information is perfectly acceptable, but knowing all the relevant details before adopting a final design is (also) a good idea.</p>



<a name="195867150"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195867150" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195867150">(Apr 30 2020 at 16:43)</a>:</h4>
<p><span class="user-mention" data-user-id="296355">@Rich Kadel</span> okay, what's the granularity then? expressions nest arbitrarily, do you want to instrument at every point in the expression tree? or is it coarser than that?</p>



<a name="195867265"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195867265" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195867265">(Apr 30 2020 at 16:44)</a>:</h4>
<p>having spent time looking into this before, I don't see how you can efficiently instrument code without using a control-flow graph</p>



<a name="195867325"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195867325" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195867325">(Apr 30 2020 at 16:44)</a>:</h4>
<p><span class="user-mention silent" data-user-id="119009">eddyb</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195867150" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195867150">said</a>:</p>
<blockquote>
<p><span class="user-mention silent" data-user-id="296355">Rich Kadel</span> okay, what's the granularity then? expressions nest arbitrarily, do you want to instrument at every point in the expression tree? or is it coarser than that?</p>
</blockquote>
<p>Every coded branch.</p>



<a name="195867388"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195867388" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195867388">(Apr 30 2020 at 16:45)</a>:</h4>
<p>so you want to find all of the control-flow edges in the AST?</p>



<a name="195867670"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195867670" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> bjorn3 <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195867670">(Apr 30 2020 at 16:47)</a>:</h4>
<p>If you instrument right before every MIR <code>SwitchInt</code>, <code>Call</code>, <code>Drop</code> and <code>Assert</code> terminator, you will get every possible branch. If you want to ignore unwinds, you only need to instrument <code>SwitchInt</code>.</p>



<a name="195867672"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195867672" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195867672">(Apr 30 2020 at 16:47)</a>:</h4>
<p>we used to have code that did that (extract a CFG from an AST). we've recently removed it after a successful transition to MIR borrowck</p>



<a name="195868219"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195868219" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195868219">(Apr 30 2020 at 16:49)</a>:</h4>
<p><span class="user-mention" data-user-id="296355">@Rich Kadel</span> okay let me ask a more useful question: say you know all the points that are relevant to control-flow: how do you represent all of the code in between, for coverage purposes?</p>



<a name="195868304"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195868304" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195868304">(Apr 30 2020 at 16:50)</a>:</h4>
<p>based on the answer to this, the solution may even be implementable all the way down in codegen, even after MIR optimizations have run</p>



<a name="195868426"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195868426" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195868426">(Apr 30 2020 at 16:50)</a>:</h4>
<p>with the pre-existing information (which mainly exists for debuginfo)</p>



<a name="195868673"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195868673" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195868673">(Apr 30 2020 at 16:52)</a>:</h4>
<p>IIRC, the LLVM format is more fine-grained than lines, and uses some sort of (nested?) range system, but it's been two years since I've looked at it</p>



<a name="195868783"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195868783" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195868783">(Apr 30 2020 at 16:52)</a>:</h4>
<p><span class="user-mention silent" data-user-id="119009">eddyb</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195866847" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195866847">said</a>:</p>
<blockquote>
<p>I mean for implementing this feature, not using it</p>
</blockquote>
<p>That's what I mean too. But I think you are arguing for injecting coverage without caring about what the input source looked like, with the (probably reasonable) assumption that using a downstream representation will be "more efficient" (as you put it), and that's fair. You are much more experienced with this than I am.</p>
<p>All I can say is, I believe I can identify coverage regions and inject coverage counters based on AST nodes without needing to know much more than what Rust source looks like.</p>
<p>I certainly don't want to argue with you over details you are much more familiar with than I am. Just trying to be practical, given my current knowledge.</p>



<a name="195869028"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195869028" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195869028">(Apr 30 2020 at 16:54)</a>:</h4>
<p>I'm arguing for something that we'd want to maintain long-term, <em>without loss of precision</em></p>



<a name="195869153"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195869153" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195869153">(Apr 30 2020 at 16:55)</a>:</h4>
<p>and I want to understand what parts of source code you care about, that the MIR doesn't already represent accurately</p>



<a name="195869205"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195869205" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195869205">(Apr 30 2020 at 16:55)</a>:</h4>
<p>after all, our debuginfo is emitted from MIR, <em>and in a lossy way</em>. that is, MIR has more information than DWARF can encode</p>



<a name="195869526"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195869526" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195869526">(Apr 30 2020 at 16:57)</a>:</h4>
<p><span class="user-mention" data-user-id="296355">@Rich Kadel</span> anyway to the best of my knowledge there is no advantage to doing the injection at any point before HIR -&gt; MIR</p>



<a name="195869541"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195869541" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195869541">(Apr 30 2020 at 16:57)</a>:</h4>
<p><span class="user-mention silent" data-user-id="119009">eddyb</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195868219" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195868219">said</a>:</p>
<blockquote>
<p><span class="user-mention silent" data-user-id="296355">Rich Kadel</span> okay let me ask a more useful question: say you know all the points that are relevant to control-flow: how do you represent all of the code in between, for coverage purposes?</p>
</blockquote>
<p>I can try an example</p>



<a name="195869698"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195869698" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195869698">(Apr 30 2020 at 16:58)</a>:</h4>
<p><span class="user-mention silent" data-user-id="119009">eddyb</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195869526" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195869526">said</a>:</p>
<blockquote>
<p><span class="user-mention silent" data-user-id="296355">Rich Kadel</span> anyway to the best of my knowledge there is no advantage to doing the injection at any point before HIR -&gt; MIR</p>
</blockquote>
<p>What do you mean by "advantage", just so I'm clear?</p>



<a name="195869722"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195869722" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> tmandry <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195869722">(Apr 30 2020 at 16:58)</a>:</h4>
<p><span class="user-mention silent" data-user-id="119009">eddyb</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195869153" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195869153">said</a>:</p>
<blockquote>
<p>and I want to understand what parts of source code you care about, that the MIR doesn't already represent accurately</p>
</blockquote>
<p>I think this is the point that's unclear right now. We don't know if there's any loss of fidelity in the MIR representation, and it would take some work to find out. The most important thing is full fidelity, so from that standpoint I can see why AST or HIR feel safest. But your earlier point about this producing messy MIR with a lot of unwind branches is well taken (by me).. that will impact compiler performance quite a lot.</p>



<a name="195869782"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195869782" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195869782">(Apr 30 2020 at 16:58)</a>:</h4>
<p>not only will it impact performance, it risks impacting <em>semantics</em></p>



<a name="195869816"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195869816" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195869816">(Apr 30 2020 at 16:59)</a>:</h4>
<p>which is far scarier than code coverage not being 100% accurate</p>



<a name="195869859"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195869859" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195869859">(Apr 30 2020 at 16:59)</a>:</h4>
<p><span class="user-mention silent" data-user-id="119009">eddyb</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195869782" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195869782">said</a>:</p>
<blockquote>
<p>not only will it impact performance, it risks impacting <em>semantics</em></p>
</blockquote>
<p>I don't believe the injection I have in mind will impact semantics at all, btw</p>



<a name="195870019"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195870019" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195870019">(Apr 30 2020 at 17:00)</a>:</h4>
<p>you're changing the HIR which goes into type-checking, there are enough subtleties that it's hard to say just from looking at it</p>



<a name="195870108"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195870108" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195870108">(Apr 30 2020 at 17:00)</a>:</h4>
<p>the only thing I am aware of that is almost guaranteed to not impact semantics is adding non-<code>let</code> statements</p>



<a name="195870206"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195870206" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195870206">(Apr 30 2020 at 17:01)</a>:</h4>
<p>in that it translates to adding a sub-CFG of MIR that doesn't change the CFG around it</p>



<a name="195870257"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195870257" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195870257">(Apr 30 2020 at 17:01)</a>:</h4>
<p>well, a statement that doesn't mention any local variables, that is</p>



<a name="195870344"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195870344" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195870344">(Apr 30 2020 at 17:02)</a>:</h4>
<p>it's equivalent to having a new built-in MIR statement, and can't influence typeck of the code around it either</p>



<a name="195870412"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195870412" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195870412">(Apr 30 2020 at 17:02)</a>:</h4>
<p>anyway, AST -&gt; HIR is "lossy except for diagnostics", in that we actually preserve enough information, despite desugaring the structure, to know what kind of AST shape some HIR came from</p>



<a name="195870491"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195870491" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195870491">(Apr 30 2020 at 17:03)</a>:</h4>
<p><span class="user-mention" data-user-id="119009">@eddyb</span> you type too fast and I don't have time to respond :-)</p>



<a name="195870609"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195870609" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195870609">(Apr 30 2020 at 17:04)</a>:</h4>
<p>time is money :P</p>



<a name="195870644"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195870644" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195870644">(Apr 30 2020 at 17:04)</a>:</h4>
<p>so while you can inject in AST -&gt; HIR, you can also do it as HIR -&gt; HIR (except for HIR transformations not existing), which means you can do it in HIR -&gt; MIR</p>



<a name="195870754"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195870754" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195870754">(Apr 30 2020 at 17:05)</a>:</h4>
<p>doing it as late as possible using the same information source means there is less friction/interference with other parts of the compiler</p>



<a name="195870848"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195870848" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195870848">(Apr 30 2020 at 17:06)</a>:</h4>
<p>I get that and am on board generally speaking.</p>



<a name="195870852"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195870852" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195870852">(Apr 30 2020 at 17:06)</a>:</h4>
<p>then the only question about MIR specifically is whether MIR -&gt; MIR is lossier compared to HIR -&gt; MIR</p>



<a name="195870992"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195870992" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195870992">(Apr 30 2020 at 17:07)</a>:</h4>
<p>but if it starts off in HIR -&gt; MIR then it's much easier to move later to MIR -&gt; MIR, even if there are reasons to believe we can't do it in MIR -&gt; MIR from the start, or just because we don't want to bother</p>



<a name="195871141"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195871141" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195871141">(Apr 30 2020 at 17:08)</a>:</h4>
<p>and to avoid constructing true calls, you can add a new <code>StatementKind</code></p>



<a name="195871148"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195871148" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eggyal <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195871148">(Apr 30 2020 at 17:08)</a>:</h4>
<p>Perhaps I can add something useful here.  I was working on this very issue back towards the end of last year, but events rather swamped over me and I've not been able to return to it.  I'm delighted to see <span class="user-mention" data-user-id="296355">@Rich Kadel</span> picking it up and taking it forward.  On the issue of when to instrument, I had preferred instrumenting the MIR for all the reasons that are being articulated here now—but...</p>



<a name="195871499"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195871499" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eggyal <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195871499">(Apr 30 2020 at 17:11)</a>:</h4>
<p>The LLVM instrumentation leans toward having calculated regions wherever possible, and only making instrumentation calls where the count cannot be imputed from other counters.  I found it impossible to reliably formulate the necessary flow graph from the MIR, and saw this as something far easier to construct with the HIR.</p>



<a name="195871550"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195871550" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195871550">(Apr 30 2020 at 17:11)</a>:</h4>
<p>(note that HIR -&gt; MIR is <em>not</em> MIR instrumentation, but HIR instrumentation, just emitting the instrumentation calls in MIR instead of trying to fit them into HIR)</p>



<a name="195871691"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195871691" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195871691">(Apr 30 2020 at 17:12)</a>:</h4>
<p><span class="user-mention" data-user-id="249694">@eggyal</span> hmm I would've used linear sequences of basic blocks that are connected by<code>Call</code> terminators, but that's just an optimization over using basic blocks</p>



<a name="195871786"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195871786" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eggyal <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195871786">(Apr 30 2020 at 17:13)</a>:</h4>
<p><span class="user-mention" data-user-id="119009">@eddyb</span> But that doesn't help you decide when a linear sequence has no other entry points?</p>



<a name="195871792"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195871792" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195871792">(Apr 30 2020 at 17:13)</a>:</h4>
<p>unless you mean considering each basic block's counter a variable in a system of equations and trying to reduce the base number of variables</p>



<a name="195871861"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195871861" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195871861">(Apr 30 2020 at 17:14)</a>:</h4>
<p><span class="user-mention" data-user-id="249694">@eggyal</span> what do you mean, that's just a property of the CFG</p>



<a name="195871958"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195871958" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195871958">(Apr 30 2020 at 17:15)</a>:</h4>
<p>you either compute that by doing a full traversal or just use the <code>predecessors</code> cache if you want</p>



<a name="195872475"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195872475" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195872475">(Apr 30 2020 at 17:19)</a>:</h4>
<p>if you do a full traversal you can just record the number of ingoing edges (i.e. predecessors) of each basic block and then you only treat it as not needing its own counter if it has exactly one ingoing edge that's e.g. not conditional</p>



<a name="195872522"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195872522" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195872522">(Apr 30 2020 at 17:20)</a>:</h4>
<p>you can do all sorts of things on a CFG</p>



<a name="195872628"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195872628" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195872628">(Apr 30 2020 at 17:20)</a>:</h4>
<p>what I'd rather be worried about with MIR -&gt; MIR is whether code affected by counters is describable given the MIR</p>



<a name="195872648"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195872648" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eggyal <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195872648">(Apr 30 2020 at 17:20)</a>:</h4>
<p>Hm.  I don't recall seeing a <code>predecessors</code> cache—so that may well have made a big difference!  I also struggled mapping some blocks back to source code regions, though that was early on in my exploration and I can't immediately recall what the issues were.  Reading back over the comments at the time, I also see <a href="https://github.com/rust-lang/rust/issues/34701#issuecomment-548578062" title="https://github.com/rust-lang/rust/issues/34701#issuecomment-548578062">@**Bob Wilson**'s comment</a></p>
<blockquote>
<p>Consider "if let condition" followed by a blank line vs. "match condition" followed by a blank line. What execution count should be displayed for that blank line? For the "if", Clang shows the "then" count. The corresponding behavior for the "match" (or a match lowered from "if let") would be to show the count for the first arm of the match. That just seems wrong to me.</p>
</blockquote>
<p>These sorts of gap regions are difficult to identify and locate from the MIR.</p>



<a name="195872674"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195872674" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Bob Wilson <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195872674">(Apr 30 2020 at 17:21)</a>:</h4>
<p>Wow, there's a lot of discussion here today and I've only got a few minutes this morning to respond. I can follow up later with more detailed responses.</p>
<ul>
<li>The compile time and runtime overhead for both Swift and Clang is relatively low, but it depends on the input. I don't remember specific numbers, but it was good enough that we briefly considered enabling it by default in Xcode Debug builds (but eventually decided against that because of larger overheads for some projects).</li>
<li>Swift is a better comparison than Clang because the coverage data structures have to be plumbed through the SIL level before getting to LLVM IR.</li>
<li>This discussion has been focused on how to inject the LLVM intrinsic calls for instrumentation. In my experience, that is the easy part. Generating the coverage map is more complicated. It might be helpful to start with that and work backward to some of the other aspects of this. I'm still not entirely clear on how the coverage map will be correlated with the instrumentation counters.</li>
<li>I stand by my earlier statement that the coverage map should be generated from the AST, but I'm increasingly skeptical about inserting the LLVM intrinsic calls into the AST. My suggestion to Rich was to build a side table mapping AST nodes to counters, lower that through HIR, and then insert the intrinsic calls during the HIR-to-MIR lowering. This is basically what Swift does. One nice thing about Rich's proposal to modify the AST is that it keeps the coverage code in one place (at least to some extent) instead of scattering bits across different parts of the compiler. He is still optimistic that he can get it to work at the AST, but it seems much more manageable at the MIR level.</li>
<li>The Clang design for instrumentation-based PGO intentionally tracks source-visible branches. The intent is that compiler-generated control flow should be annotated with branch probabilities by the compiler. For coverage, there is no point in instrumenting branches that are not visible in the source, and I don't think that should be done here.</li>
</ul>



<a name="195872718"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195872718" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195872718">(Apr 30 2020 at 17:21)</a>:</h4>
<p><span class="user-mention" data-user-id="249694">@eggyal</span> okay yeah this is the interesting parts to me (mapping the source code)</p>



<a name="195872749"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195872749" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195872749">(Apr 30 2020 at 17:21)</a>:</h4>
<p><span class="user-mention" data-user-id="296362">@Bob Wilson</span> sorry, I tried to stay out of it but <span class="user-mention" data-user-id="116883">@tmandry</span> pinged me</p>



<a name="195872838"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195872838" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195872838">(Apr 30 2020 at 17:22)</a>:</h4>
<p><span class="user-mention" data-user-id="296362">@Bob Wilson</span> I can clear up something right away: this is unnecessary: "side table mapping AST nodes to counters, lower that through HIR, and then insert the intrinsic calls during the HIR-to-MIR lowering"</p>



<a name="195872883"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195872883" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195872883">(Apr 30 2020 at 17:22)</a>:</h4>
<p>the HIR contains enough information that you don't need to know the AST it came from</p>



<a name="195872939"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195872939" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195872939">(Apr 30 2020 at 17:23)</a>:</h4>
<p>this mostly exists for diagnostics, which need to be accurate even when the actual constructs were desugared</p>



<a name="195873135"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195873135" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195873135">(Apr 30 2020 at 17:24)</a>:</h4>
<p>also, <code>rustc</code>'s <code>Span</code>s record a macro backtrace, and HIR desugaring is also encoded in that, so you can probably do a bunch more things that not even heavy diagnostics logic does yet :P</p>



<a name="195873343"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195873343" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195873343">(Apr 30 2020 at 17:26)</a>:</h4>
<p><span class="user-mention" data-user-id="249694">@eggyal</span> I don't understand the quoted example. is the blank line inside the condition expression part of the syntax or after the <code>{</code>?</p>



<a name="195873545"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195873545" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195873545">(Apr 30 2020 at 17:27)</a>:</h4>
<p>are we talking about a blank line that's outside of a <code>match</code> arm but inside the <code>{...}</code> that enclose the match arms?</p>



<a name="195873574"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195873574" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Bob Wilson <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195873574">(Apr 30 2020 at 17:28)</a>:</h4>
<p><span class="user-mention silent" data-user-id="119009">eddyb</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195872883" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195872883">said</a>:</p>
<blockquote>
<p>the HIR contains enough information that you don't need to know the AST it came from</p>
</blockquote>
<p>I'm not that familiar with it, but I'm not convinced. The coverage map generated from the AST needs to refer to counter indices. The counter indices are associated with different AST nodes and the details on how to inject the instrumentation may depend on the type of the AST node. How can you get that counter index from HIR and what kind of node it came from (e.g., a "match" lowered from an  AST "if" should probably be instrumented differently than a source-level "match")?</p>



<a name="195873577"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195873577" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195873577">(Apr 30 2020 at 17:28)</a>:</h4>
<p><span class="user-mention" data-user-id="249694">@eggyal</span> keep in mind you have this problem all the way throughout the AST -&gt; HIR -&gt; MIR pipeline (you always have to work with <code>Span</code>s that don't necessarily touch)</p>



<a name="195873694"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195873694" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195873694">(Apr 30 2020 at 17:28)</a>:</h4>
<p><span class="user-mention" data-user-id="296362">@Bob Wilson</span> isn't <code>rustc</code>'s job to invent those counters?</p>



<a name="195873780"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195873780" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195873780">(Apr 30 2020 at 17:29)</a>:</h4>
<p>why would AST and HIR be different? other than the desugaring, which I just pointed out you can observe that it happened</p>



<a name="195873795"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195873795" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195873795">(Apr 30 2020 at 17:29)</a>:</h4>
<p>you can tell apart <code>if-let</code> from <code>match</code> on HIR</p>



<a name="195873811"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195873811" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195873811">(Apr 30 2020 at 17:29)</a>:</h4>
<p>again, mostly for diagnostics reasons</p>



<a name="195873939"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195873939" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195873939">(Apr 30 2020 at 17:30)</a>:</h4>
<p>anyway I agree that figuring out the coverage map is the most important part of this</p>



<a name="195874223"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195874223" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195874223">(Apr 30 2020 at 17:33)</a>:</h4>
<p>HIR/MIR has no(conditional branches that aren't visible in the source with two notable exceptions I guess: the pattern-matching on <code>Iterator::next</code>'s result in <code>for</code> loop, and the pattern-matching <code>?</code> does. but MIR can tell they came from a desugaring, last I checked</p>



<a name="195874262"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195874262" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195874262">(Apr 30 2020 at 17:33)</a>:</h4>
<p>HIR -&gt; MIR introduces some checks for e.g. array indexing and checked arithmetic, but those use the <code>Assert</code> terminator, not conditional control-flow</p>



<a name="195874437"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195874437" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> tmandry <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195874437">(Apr 30 2020 at 17:35)</a>:</h4>
<p>Naively to me, it does seem like you can instrument every branch regardless of what the source looked like (modulo optimizing away unnecessary counters like <span class="user-mention" data-user-id="249694">@eggyal</span> was talking about), and care about what the source looked like only when building a map between counters and source code. If means multiple "hidden" branches map back to the exact same code, that's okay; that additional information could still be useful for optimization purposes.</p>



<a name="195874490"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195874490" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195874490">(Apr 30 2020 at 17:35)</a>:</h4>
<p>also, rather than gaps, I'd be more concerned about overlaps</p>



<a name="195874581"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195874581" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195874581">(Apr 30 2020 at 17:36)</a>:</h4>
<p>how do you build a coverage map out of an expression tree where each expression has a range in the source?</p>



<a name="195874712"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195874712" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195874712">(Apr 30 2020 at 17:37)</a>:</h4>
<p>what's the algorithm? how does it behave when the naive assumption that the ranges are trivially nested, breaks down?</p>



<a name="195874765"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195874765" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195874765">(Apr 30 2020 at 17:37)</a>:</h4>
<p>how do you handle all of the ways in which macro expansion can create spans?</p>



<a name="195874867"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195874867" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eggyal <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195874867">(Apr 30 2020 at 17:38)</a>:</h4>
<p><span class="user-mention silent" data-user-id="119009">eddyb</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195874712" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195874712">said</a>:</p>
<blockquote>
<p>what's the algorithm? how does it behave when the naive assumption that the ranges are trivially nested, breaks down?</p>
</blockquote>
<p>It's worth reading <a href="http://llvm.org/docs/CoverageMappingFormat.html#mapping-region" title="http://llvm.org/docs/CoverageMappingFormat.html#mapping-region">http://llvm.org/docs/CoverageMappingFormat.html#mapping-region</a></p>



<a name="195874883"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195874883" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195874883">(Apr 30 2020 at 17:38)</a>:</h4>
<p>and I have, two years ago :)</p>



<a name="195874894"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195874894" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195874894">(Apr 30 2020 at 17:38)</a>:</h4>
<p>my question is how do you produce that <em>from Rust ASTs</em></p>



<a name="195875166"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195875166" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195875166">(Apr 30 2020 at 17:40)</a>:</h4>
<p>the best way to handle gaps, IMO, is to consider anything that's not a sub-range, as always executing if the parent expression executes. this means that various bits of syntax, punctuation, etc., are automatically included, and the only parts that can vary dynamically are exactly what's not guaranteed to also execute</p>



<a name="195875285"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195875285" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195875285">(Apr 30 2020 at 17:41)</a>:</h4>
<p>AFAIK the format allows this and many other approaches, but you still have to pick a way to turn expression spans into that format</p>



<a name="195875522"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195875522" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195875522">(Apr 30 2020 at 17:43)</a>:</h4>
<p>My feeling is the discussion on where to include whitespace, comments, and punctuation are very minor issues and maybe not worth too much discussion at this stage of the proposal.</p>



<a name="195875525"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195875525" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195875525">(Apr 30 2020 at 17:43)</a>:</h4>
<p>the nice thing about a sufficiently capable algorithm  is that it will likely be correct when throwing MIR spans at it willy nilly</p>



<a name="195875572"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195875572" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195875572">(Apr 30 2020 at 17:43)</a>:</h4>
<p><span class="user-mention" data-user-id="296355">@Rich Kadel</span> a correct implementation wrt those is likely going to be most of the effort</p>



<a name="195875724"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195875724" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195875724">(Apr 30 2020 at 17:44)</a>:</h4>
<p>anyway when I agreed to offer <span class="user-mention" data-user-id="116883">@tmandry</span> my emails from two years ago I didn't agree to repeat the whole experience again</p>



<a name="195875812"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195875812" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195875812">(Apr 30 2020 at 17:45)</a>:</h4>
<p>my main piece of advice is that there shouldn't be any loss of information between the AST observed by AST -&gt; HIR and the HIR observed by HIR -&gt; MIR and injecting in the latter is less likely to be a problem long-term</p>



<a name="195875977"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195875977" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195875977">(Apr 30 2020 at 17:47)</a>:</h4>
<p>HIR -&gt; MIR vs MIR -&gt; MIR is a more advanced matter of syntactically shallow vs semantic approaches, and what the actual goals are</p>



<a name="195875989"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195875989" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195875989">(Apr 30 2020 at 17:47)</a>:</h4>
<p><span class="user-mention silent" data-user-id="119009">eddyb</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195875724" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195875724">said</a>:</p>
<blockquote>
<p>anyway when I agreed to offer <span class="user-mention silent" data-user-id="116883">tmandry</span> my emails from two years ago I didn't agree to repeat the whole experience again</p>
</blockquote>
<p>Apologies if I've cause some churn here. Was not aware of these emails, but I can get them from <span class="user-mention" data-user-id="116883">@tmandry</span> </p>
<p>Thanks for the great feedback.</p>



<a name="195876016"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195876016" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195876016">(Apr 30 2020 at 17:47)</a>:</h4>
<p>ah I thought tmandry already discussed it</p>



<a name="195876133"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195876133" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195876133">(Apr 30 2020 at 17:48)</a>:</h4>
<p>anyway I will try to stay out of the way of this. I cannot be 100% objective due to the stuff from a couple years ago</p>



<a name="195876176"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195876176" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195876176">(Apr 30 2020 at 17:48)</a>:</h4>
<p>most of the information is in what I said above and those emails, although they might be outdated by now</p>



<a name="195876222"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195876222" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195876222">(Apr 30 2020 at 17:49)</a>:</h4>
<p><span class="user-mention" data-user-id="133247">@bjorn3</span>, <span class="user-mention" data-user-id="125250">@Wesley Wiser</span> and the rest of <span class="user-group-mention" data-user-group-id="1162">@WG-mir-opt</span> should be able to help</p>



<a name="195876260"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195876260" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195876260">(Apr 30 2020 at 17:49)</a>:</h4>
<p>with either HIR -&gt; MIR or MIR -&gt; MIR</p>



<a name="195876333"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195876333" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> eddyb <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195876333">(Apr 30 2020 at 17:49)</a>:</h4>
<p>good luck, have fun, stay safe, etc.</p>



<a name="195876682"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195876682" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195876682">(Apr 30 2020 at 17:52)</a>:</h4>
<p>Thank you! You as well.</p>



<a name="195887451"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195887451" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> tmandry <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195887451">(Apr 30 2020 at 19:11)</a>:</h4>
<p>(deleted)</p>



<a name="195903383"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/195903383" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Bob Wilson <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#195903383">(Apr 30 2020 at 21:22)</a>:</h4>
<p>It sounds like there is some history about this topic that I'm not aware of.... Anyway, I generally agree with @eddyb, but I still think building the coverage map from ASTs would be more maintainable, since doing it from HIR would need to be updated to account for any changes in the desugaring over time. If you produce the coverage map from the AST, it shouldn't be hard to pass it through HIR -- it's mostly self-contained except for the mapping to counter numbers, and I think that mapping could be translated to describe the HIR without much difficulty. Inserting instrumentation during HIR-&gt;MIR lowering would also be my recommendation. <span class="user-mention" data-user-id="296355">@Rich Kadel</span>, you don't really need to understand MIR very well to do that, since the analysis of the control flow would be done earlier.</p>



<a name="196254406"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/196254406" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Bob Wilson <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#196254406">(May 05 2020 at 00:01)</a>:</h4>
<p><span class="user-mention" data-user-id="296355">@Rich Kadel</span> You wrote in the GitHub PR and issue that you're now planning to implement this as a MIR -&gt; MIR pass based on feedback here. From what I see here, the clear feedback is that the instrumentation should be inserted in MIR, but there was at least as many recommendations to do that in HIR -&gt; MIR lowering, with the coverage map computed from the AST or HIR. What led you to settle on MIR -&gt; MIR? Specifically, how do you intend to build the coverage map from MIR? I saw a few comments suggesting that is should be doable, but it still sounds difficult to me.</p>



<a name="196255224"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/196255224" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#196255224">(May 05 2020 at 00:11)</a>:</h4>
<p><span class="user-mention" data-user-id="296362">@Bob Wilson</span> I did a walkthrough of the MIR with <span class="user-mention" data-user-id="116883">@tmandry</span> on Friday and I can see in the CFG and data structures how the MIR branches <em>should</em> map back to the AST nodes; and from there, I expect to be able to map back to the coverage regions. This still has to be proven of course, but MIR-&gt;MIR was cleary @eddyb 's preferred approach, and based on his experience, and <span class="user-mention" data-user-id="116883">@tmandry</span> 's recommendations (which I believe were influenced by the discussion in this thread), it's worth prioritizing an attempt at MIR-&gt;MIR. If I run into any unexpected complications, I'll document them and can discuss the pros and cons of alternatives from there.</p>



<a name="196256019"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/196256019" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#196256019">(May 05 2020 at 00:23)</a>:</h4>
<p><span class="user-mention silent" data-user-id="119009">eddyb</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195876222" title="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/195876222">said</a>:</p>
<blockquote>
<p><span class="user-mention silent" data-user-id="133247">bjorn3</span>, <span class="user-mention silent" data-user-id="125250">Wesley Wiser</span> and the rest of <span class="user-group-mention" data-user-group-id="1162">@WG-mir-opt</span> should be able to help</p>
</blockquote>
<p><span class="user-group-mention" data-user-group-id="1162">@WG-mir-opt</span> Please take a look at my revisions to the <a href="https://github.com/rust-lang/compiler-team/issues/278" title="https://github.com/rust-lang/compiler-team/issues/278">Rust coverage MCP submission</a>. (I tried to tag this group by @mention but github does not allow me to; hence this ping.)  Thanks!</p>



<a name="196310930"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/196310930" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Wesley Wiser <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#196310930">(May 05 2020 at 13:59)</a>:</h4>
<p>Thanks for the ping <span class="user-mention" data-user-id="296355">@Rich Kadel</span>! I'm going to spend some time this evening going over the revised proposal and the rest of the thread here.</p>



<a name="196652538"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/196652538" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> triagebot <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#196652538">(May 06 2020 at 15:36)</a>:</h4>
<p><span class="user-group-mention" data-user-group-id="492">@T-compiler</span>: Proposal <a href="https://github.com/rust-lang/compiler-team/issues/278#issuecomment-624722653" title="https://github.com/rust-lang/compiler-team/issues/278#issuecomment-624722653">#278</a> has been seconded, and will be approved in 10 days if no objections are raised.</p>



<a name="198372848"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/198372848" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#198372848">(May 21 2020 at 20:28)</a>:</h4>
<p><span class="user-group-mention" data-user-group-id="492">@T-compiler</span>: Proposal <a href="https://github.com/rust-lang/compiler-team/issues/278#issuecomment-624722653">#278</a> was accepted/approved, with note from <span class="user-mention" data-user-id="116009">@nikomatsakis</span> on May 21. Thanks!</p>



<a name="206731748"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/206731748" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#206731748">(Aug 12 2020 at 18:23)</a>:</h4>
<p><span class="user-group-mention" data-user-group-id="492">@T-compiler</span> - I'd like to give the broader compiler team an opportunity to take a look at and potentially weigh in on a suggested change to how I represent coverage counter metadata injected into MIR.</p>
<p>I currently encode metadata as Rust intrinsic functions and args (Operands), most of which inform the codegen process (e.g., to generate the coverage map data section), but not always generating an actual LLVM intrinsic call.</p>
<p>The use of existing MIR Call Terminators and arguments allows me to implement the coverage instrumentation purely through MIR manipulation, but it's a bit "hacky" using "fake" function calls, and encoding and decoding <code>Operand</code>s just to pass data from the MIR phase to the codegen phase.  (This was a known tradeoff of the current approach.)</p>
<p>I recently noticed that the current approach also results in some less than optimal LLVM IR output, adding some unnecessary BasicBlocks and <code>br</code>anches, and worse, moving those blocks around so the LLVM IR is not as sequential as it would be otherwise (making it less "readable", for those that care to read it).</p>
<p>I see an opportunity to not only improve the LLVM IR generation, but also significantly improve some of the MIR and codegen aspects of this implementation.</p>
<p>I suggest changing the implementation from using "Call" terminators to<br>
using a custom <code>StatementKind</code>. This would have several benefits:</p>
<ol>
<li>One problem with using a Call <code>Terminator</code> is a <code>Terminator</code> requires its own<br>
   BasicBlock, but as it turns out, LLVM's <code>instrprof.increment</code> intrinsic is converted<br>
   into a set of inline statements that don't actually perform a function call and don't<br>
   need to unwind or branch. However, the existence of the BasicBlock in the Rust MIR CFG<br>
   results the generation of an additional BasicBlock label, and an unnecessary <code>br</code>anch,<br>
   from the statement just after the increment statements to the label for the next<br>
   block, even though the first statement of the next block is sequentionally next<br>
   anyway. Here's an LLVM IR snippet to demonstrate this. The example function has no<br>
   statements:</li>
</ol>
<h1>No instrumentation:</h1>
<div class="codehilite"><pre><span></span><code>     <span class="p">{</span>
       <span class="nl">start:</span>
       <span class="k">ret</span> <span class="k">void</span>
     <span class="p">}</span>
</code></pre></div>


<h1>Current instrumentation with required new block:</h1>
<div class="codehilite"><pre><span></span><code>     <span class="p">{</span>
       <span class="nl">start:</span>
       <span class="nv">%pgocount</span> <span class="p">=</span> <span class="k">load</span> <span class="k">i64</span><span class="p">,</span> <span class="k">i64</span><span class="p">*</span> <span class="k">getelementptr</span> <span class="k">inbounds</span> <span class="p">([</span><span class="m">1</span> <span class="k">x</span> <span class="k">i64</span><span class="p">],</span>  <span class="p">[</span><span class="m">1</span> <span class="k">x</span> <span class="k">i64</span><span class="p">]*</span> <span class="vg">@__profc_testfunc</span><span class="p">,</span> <span class="k">i64</span> <span class="m">0</span><span class="p">,</span> <span class="k">i64</span> <span class="m">0</span><span class="p">)</span>
       <span class="nv nv-Anonymous">%0</span> <span class="p">=</span> <span class="k">add</span> <span class="k">i64</span> <span class="nv">%pgocount</span><span class="p">,</span> <span class="m">1</span>
       <span class="k">store</span> <span class="k">i64</span> <span class="nv nv-Anonymous">%0</span><span class="p">,</span> <span class="k">i64</span><span class="p">*</span> <span class="k">getelementptr</span> <span class="k">inbounds</span> <span class="p">([</span><span class="m">1</span> <span class="k">x</span> <span class="k">i64</span><span class="p">],</span> <span class="p">[</span><span class="m">1</span> <span class="k">x</span> <span class="k">i64</span><span class="p">]*</span> <span class="vg">@__profc_testfunc</span><span class="p">,</span> <span class="k">i64</span> <span class="m">0</span><span class="p">,</span> <span class="k">i64</span> <span class="m">0</span><span class="p">)</span>
       <span class="k">br</span> <span class="k">label</span> <span class="nv">%bb1</span>          <span class="c">; unnecessary</span>

       <span class="nl">bb1:</span>                   <span class="c">; unnecessary</span>
       <span class="k">ret</span> <span class="k">void</span>
     <span class="p">}</span>
</code></pre></div>


<h1>Expected instrumentation, possible with custom coverage <code>StatementKind</code></h1>
<div class="codehilite"><pre><span></span><code>     <span class="p">{</span>
       <span class="nl">start:</span>
       <span class="nv">%pgocount</span> <span class="p">=</span> <span class="k">load</span> <span class="k">i64</span><span class="p">,</span> <span class="k">i64</span><span class="p">*</span> <span class="k">getelementptr</span> <span class="k">inbounds</span> <span class="p">([</span><span class="m">1</span> <span class="k">x</span> <span class="k">i64</span><span class="p">],</span>  <span class="p">[</span><span class="m">1</span> <span class="k">x</span> <span class="k">i64</span><span class="p">]*</span> <span class="vg">@__profc_testfunc</span><span class="p">,</span> <span class="k">i64</span> <span class="m">0</span><span class="p">,</span> <span class="k">i64</span> <span class="m">0</span><span class="p">)</span>
       <span class="nv nv-Anonymous">%0</span> <span class="p">=</span> <span class="k">add</span> <span class="k">i64</span> <span class="nv">%pgocount</span><span class="p">,</span> <span class="m">1</span>
       <span class="k">store</span> <span class="k">i64</span> <span class="nv nv-Anonymous">%0</span><span class="p">,</span> <span class="k">i64</span><span class="p">*</span> <span class="k">getelementptr</span> <span class="k">inbounds</span> <span class="p">([</span><span class="m">1</span> <span class="k">x</span> <span class="k">i64</span><span class="p">],</span> <span class="p">[</span><span class="m">1</span> <span class="k">x</span> <span class="k">i64</span><span class="p">]*</span> <span class="vg">@__profc_testfunc</span><span class="p">,</span> <span class="k">i64</span> <span class="m">0</span><span class="p">,</span> <span class="k">i64</span> <span class="m">0</span><span class="p">)</span>
       <span class="k">ret</span> <span class="k">void</span>
     <span class="p">}</span>
</code></pre></div>


<ol start="2">
<li>
<p>Also, to inject the new BasicBlock requires adding the block to the end of the vector<br>
   of BasicBlocks, and then, since the new block has to be inserted BEFORE another block,<br>
   the "next" BasicBlockData has to be swapped with the new intrinsic Call terminator<br>
   BasicBlockData, which results in LLVM IR that jumps around more than it would<br>
   otherwise. (What was the first block in a Function moves to the last block in the<br>
   function, and shows up last in LLVM IR, with a <code>br</code>anch to that last block after<br>
   incrementing the intrinsic. A custom <code>StatementKind</code> would avoid all of this.</p>
</li>
<li>
<p>A custom statement should allow us to embed metadata in the new StatementKind(s)<br>
   without requiring me to encode them as <code>Operand</code>s, for which the translations can be<br>
   somewhat obscure. Currently, the <code>Operand</code> types require very specific <code>Ty</code>pe<br>
   metadata that was really hard to get right, just to make the intrinsics look like<br>
   normal Rust function calls, even though they are never actually used that way.</p>
</li>
<li>
<p>The filename <code>str</code> generates an <code>Allocation</code> for the string content, and even though<br>
   this const operand is _only_ used pre-codegen, this unused alloc still gets injected<br>
   into LLVM IR. This entire issue can be avoided with a custom <code>StatementKind</code>.</p>
</li>
<li>
<p>The temporary objects and corresponding <code>StorageLive</code> and <code>StorageDead</code> <code>StatementKinds</code>,<br>
   required by the <code>Call</code> as a return result (though empty), can be removed.</p>
</li>
<li>
<p>All of the essentially "fake" coverage intrinsic function declarations, and the<br>
   associated "lang items" can be removed.</p>
</li>
<li>
<p>The special handling of these calls during code generation can be migrated to more<br>
   isolated and coverage-specific handlers of the coverage <code>StatementKind</code>(s), which would<br>
   make the rest of the intrinsic codegen handling less confusing.</p>
</li>
</ol>
<p>All of these changes will probably result in a simpler and smaller overall<br>
implementation.</p>
<p>(Attn to primary reviewers: <span class="user-mention" data-user-id="125250">@Wesley Wiser</span> <span class="user-mention" data-user-id="116883">@tmandry</span> )</p>



<a name="206733352"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/206733352" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> tmandry <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#206733352">(Aug 12 2020 at 18:37)</a>:</h4>
<p>Given that the eventual goal is to add an intrinsic to every branch, I think adding another <code>StatementKind</code> makes sense given the likely performance impact and the complexity of the IR that we'd get otherwise.</p>



<a name="206733530"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/206733530" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> tmandry <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#206733530">(Aug 12 2020 at 18:39)</a>:</h4>
<p>That said, there of course is a cost of adding a new case to everything that consumes MIR</p>



<a name="206734028"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/206734028" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> tmandry <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#206734028">(Aug 12 2020 at 18:43)</a>:</h4>
<p>I wonder if other intrinsics would also benefit from being modeled as statements</p>



<a name="206739499"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/206739499" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#206739499">(Aug 12 2020 at 19:29)</a>:</h4>
<p>Maybe. I'm not sure they would all have similar benefits, but other intrinsics in <code>rustc_codegen_ssa::mir::block</code> that don't get passed to <code>codegen_intrinsic_call()</code> include <code>transmute</code>, <code>drop_in_place</code>, <code>caller_location</code>, and (I think) <code>simd_shuffle</code>, <code>assert_inhabited</code>, <code>assert_zero_valid</code>, and <code>assert_uninit_valid</code>. These all handle codegen a bit differently, and I don't see a lot of similarity to how coverage injection is done.</p>



<a name="206739653"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/206739653" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#206739653">(Aug 12 2020 at 19:31)</a>:</h4>
<p>IMO I think it would be hard to come up with a one-size-fits-all solution. If we can limit this discussion to the original topic, I think it will be easier to get through; and if there's agreement to move forward, it could be a model for other intrinsics to follow in the future.</p>



<a name="206882549"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/206882549" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#206882549">(Aug 13 2020 at 23:11)</a>:</h4>
<p><span class="user-mention" data-user-id="125250">@Wesley Wiser</span> I noted your thumbs-up, and I'm not seeing any objections to the suggested approach. Unless you think we should wait longer, I may start working on this tomorrow. Let me know if you disagree. Thanks!</p>



<a name="206889745"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/206889745" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Wesley Wiser <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#206889745">(Aug 14 2020 at 01:20)</a>:</h4>
<p>No objections here. I do wonder though if adding new MIR StatementKinds needs to go through an MCP.</p>
<p>I can see a few ways we could go:</p>
<ul>
<li>
<p>We add a StatementKind specifically for code coverage</p>
<ul>
<li>Pros: Has pretty obvious semantics, probably the simplest solution for this problem, might be reusable for other code coverage techs </li>
<li>Cons: Not really scalable in the long run for each intrinsic</li>
</ul>
</li>
<li>
<p>We add a StatementKind for intrinsics</p>
<ul>
<li>Pros: Would scale better, might solve the unused allocation issue for string data.</li>
<li>Cons: There might not be enough commonality between intrinsics for this to make sense. We're getting along ok as it is now so this might be overkill. </li>
</ul>
</li>
<li>
<p>We add a StatementKind for calls which cannot unwind</p>
<ul>
<li>Pros: Would be conceptually similar to how we inject the performance counter now, there might be compilation time wins since we can more eagerly remove unwind branches.  </li>
<li>Cons: To do this completely would probably be a larger change than the other two but if this is the direction we choose to go, we could start by just doing this for the code coverage intrinsic and leave the rest of the work to be done later. (I would be interested in helping with that effort)</li>
</ul>
</li>
</ul>



<a name="206890440"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/206890440" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#206890440">(Aug 14 2020 at 01:34)</a>:</h4>
<p>I'm leaning toward the first option, but since there are essentially three variants of coverage intrinsics right now (increment, expression, and unreachable) I think I would implement it as a StatementKind variant with sub-variants anyway. If (perhaps deciding during code review) we decide it makes sense to make this more general purpose, we can add more sub-variants for any of those other intrinsics that make sense.</p>



<a name="206890481"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/206890481" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#206890481">(Aug 14 2020 at 01:35)</a>:</h4>
<p>That would limit the impact of adding new subvariants to a much smaller code footprint, compared with adding more StatementKinds</p>



<a name="206890835"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/206890835" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#206890835">(Aug 14 2020 at 01:43)</a>:</h4>
<p>As for going through another MCP, obviously I can't make that call, but I don't see why this would not be considered just a design decision under the existing, approved MCP. There wasn't anything in the MCP that contradicts this approach. (We even considered it in the beginning. At least, I remember Tyler suggesting using a Statement instead of inserting a block, so he gets the credit for that good idea ;-)</p>
<p>Perhaps let's see what the PR looks like, and if it feels "major", consider either looping in others for review, or consider an MCP if there are broader impacts we don't forsee.</p>



<a name="206979224"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/206979224" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Wesley Wiser <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#206979224">(Aug 14 2020 at 20:39)</a>:</h4>
<p>I'm fine with starting the work now and seeing what it looks like. There are large number of consumers of MIR in the compiler ecosystem so adding new types of statements might be disruptive enough that it requires it's own MCP. I will reach out to some folks and see what they think. If that is the case, having a PR with the suggested design would probably make it go faster so that would still be helpful.</p>



<a name="207040804"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/207040804" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#207040804">(Aug 15 2020 at 21:45)</a>:</h4>
<p>The PR is complete and ready for review: <a href="https://github.com/rust-lang/rust/pull/75563">https://github.com/rust-lang/rust/pull/75563</a></p>



<a name="207055387"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/207055387" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#207055387">(Aug 16 2020 at 05:02)</a>:</h4>
<p>I'm really happy with the results, both in terms of the rustc implementation code itself (significantly reduced SLOC and complexity), and much smaller impact on the generated MIR and LLVM IR. See for example the <a href="https://github.com/rust-lang/rust/blob/7166355861bc959cac468091da13bba8c17bc33d/src/test/mir-opt/instrument_coverage.main.InstrumentCoverage.diff">cleaner mir-opt diff</a>, vs. <a href="https://github.com/rust-lang/rust/blob/master/src/test/mir-opt/instrument_coverage.main.InstrumentCoverage.diff">the prior implementation</a>. And the unwanted <code>alloc</code> in the LLVM IR (for the non-codegenned filename string operand) is taken care of now too.</p>



<a name="207168686"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/207168686" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#207168686">(Aug 17 2020 at 16:59)</a>:</h4>
<p>I realized a couple of additional advantages (in addition to the <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/206731748">original 7 advantages I expected</a>):</p>
<ul>
<li>Diff-based tests of both MIR and LLVM are much less brittle because the lines changed have a direct relationship with the enabled coverage instrumentation feature. This will make a huge difference when I add more tests to cover different branching scenarios (if-else, match, lazy boolean, loops) because unlike the prior implementation, I don't have to move entire blocks around.</li>
<li>Since I'm not limited to primitive data types supported by Operand, I was able to take advantage of semantically-enforced <code>newtype_index!</code> types throughout the implementation, from injection to codegen to the coverage map generation. The implementation has better consistency across the board, and should be much more maintainable.</li>
</ul>



<a name="207208336"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/207208336" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Rich Kadel <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#207208336">(Aug 17 2020 at 23:22)</a>:</h4>
<p><span class="user-mention silent" data-user-id="125250">Wesley Wiser</span> <a href="#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278/near/206889745">said</a>:</p>
<blockquote>
<p>...<br>
I can see a few ways we could go:</p>
<ul>
<li>We add a <code>StatementKind</code> specifically for code coverage<br>
...</li>
</ul>
</blockquote>
<p>The PR currently calls it <code>StatementKind::Coverage</code> and has one variant value of type <code>Coverage</code>.</p>
<p>If we want to make this a little more generic, I'd like to suggest: <code>StatementKind::Injected</code>, taking an <code>InjectedKind</code> enum value. <code>Coverage</code> can then be a <code>kind</code> of <code>Injected</code> statement. </p>
<p>The semantics of an "Injected" statement would be, the statement is injected during compilation (it does not exist at the source level), meaning it can be ignored or removed without affecting the semantics of the original source code.</p>
<p>I think it's reasonable to expect an <code>Injected</code> statement does not return a value, and does not unwind.</p>
<p>I haven't looked into whether these semantics support any of the other non-codegenned intrinsics, but the <code>Injected</code> variant could be used with other, future instrumentation features (maybe similar to LLVM sanitizers).</p>
<p>This lines up with the PR feedback from <span class="user-mention" data-user-id="120791">@RalfJ</span> regarding how Miri should handle <code>Coverage</code>.</p>



<a name="216420217"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/216420217" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> tmandry <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#216420217">(Nov 12 2020 at 02:13)</a>:</h4>
<p>I put up a PR for a blog post announcing this feature: <a href="https://github.com/rust-lang/blog.rust-lang.org/pull/725">https://github.com/rust-lang/blog.rust-lang.org/pull/725</a></p>



<a name="216439271"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/216439271" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> flip1995 <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#216439271">(Nov 12 2020 at 08:27)</a>:</h4>
<p>FYI: <a href="#narrow/stream/257328-clippy/topic/Clippy.20code.20coverage/near/216439076">Clippy code coverage</a></p>



<a name="216439358"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/216439358" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> flip1995 <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#216439358">(Nov 12 2020 at 08:28)</a>:</h4>
<p>One question: Is it possible to exlude deps when creating or showing the coverage report?</p>



<a name="216524120"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/216524120" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> tmandry <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#216524120">(Nov 12 2020 at 20:33)</a>:</h4>
<p><span class="user-mention" data-user-id="264664">@flip1995</span> I think you can compile the deps without <code>-Zinstrument-coverage</code>, though it might require cargo support</p>



<a name="216524285"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/216524285" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> tmandry <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#216524285">(Nov 12 2020 at 20:34)</a>:</h4>
<p><span class="user-mention" data-user-id="296355">@Rich Kadel</span> do you know how that would interact with e.g. generic functions from other crates that are monomorphized when compiling with <code>-Zinstrument-coverage</code>?</p>



<a name="216661041"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/216661041" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Andrew Chin (eminence) <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#216661041">(Nov 13 2020 at 19:31)</a>:</h4>
<p>Hi all -- i'm having trouble finding the <code>llvm-cov</code> binary.  according to the docs, it should be next to <code>llvm-profdata</code>, but i'm not finding it</p>



<a name="216661504"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/216661504" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Andrew Chin (eminence) <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#216661504">(Nov 13 2020 at 19:33)</a>:</h4>
<p>ooh, i see.  i need <a href="https://github.com/rust-lang/rust/issues/78947">#78947</a>.  ok, nevermind!  sorry for the noise</p>



<a name="217006052"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/217006052" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Thomas McGuire <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#217006052">(Nov 17 2020 at 14:07)</a>:</h4>
<p>I had the same issue about <code>llvm-cov</code>. Maybe it's worth extending <a href="https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/source-based-code-coverage.html">https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/source-based-code-coverage.html</a> to mention how to install <code>llvm-cov</code> with <code>rustup</code>, and where to find the binary?</p>



<a name="217007716"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/217007716" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Thomas McGuire <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#217007716">(Nov 17 2020 at 14:20)</a>:</h4>
<p>Also, for anyone else reading this, in the hope it helps: When installing <code>llvm-tools-preview</code> with <code>rustup</code> for nightly, make sure to remove <code>miri</code> first, as it is not contained in the version of today (<a href="https://rust-lang.github.io/rustup-components-history/">https://rust-lang.github.io/rustup-components-history/</a>), and <code>rustup</code> will skip versions with missing components (<a href="https://github.com/rust-lang/blog.rust-lang.org/pull/725#discussion_r523181457">https://github.com/rust-lang/blog.rust-lang.org/pull/725#discussion_r523181457</a>).</p>



<a name="217012778"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/217012778" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Thomas McGuire <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#217012778">(Nov 17 2020 at 14:55)</a>:</h4>
<p>After solving the <code>llvm-cov</code> issue, it seems to work nicely so far. Good job! <span aria-label="thumbs up" class="emoji emoji-1f44d" role="img" title="thumbs up">:thumbs_up:</span></p>



<a name="217068437"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/217068437" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arnaud de Bossoreille <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#217068437">(Nov 17 2020 at 21:54)</a>:</h4>
<p>Hi, I am having trouble with the generated prof data, it is not empty but it reports 0% coverage. More details about what I do:</p>
<ul>
<li>use toolchain <code>nightly-2020-11-16</code> via <code>rust-toolchain</code> file</li>
<li>run <code>RUSTFLAGS="-Zinstrument-coverage" cargo test</code></li>
<li>run <code>llvm-profdata-10 merge -sparse default.profraw -o default.profdata</code></li>
<li>run <code>llvm-cov-10 export target/debug/deps/&lt;crate&gt;-&lt;checksum&gt; -instr-profile=default.profdata --format=lcov &gt;| lcov.txt</code></li>
<li>and finally run <code>lcov --summary lcov.txt</code> which prints</li>
</ul>
<div class="codehilite"><pre><span></span><code>Reading tracefile lcov.txt
Summary coverage rate:
  lines......: 0.0% (0 of 13741 lines)
  functions..: 0.0% (0 of 1122 functions)
  branches...: no data found
</code></pre></div>
<p>I cannot figure out what is my mistake.</p>



<a name="217072865"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Implement%20LLVM-compatible%20source-based%20cod%20compiler-team%23278/near/217072865" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arnaud de Bossoreille <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278.html#217072865">(Nov 17 2020 at 22:35)</a>:</h4>
<p>I managed to go further the doc tests overwrote the raw data after the test executable. I'll try to play with <code>LLVM_PROFILE_FILE</code>.</p>



<hr><p>Last updated: Aug 07 2021 at 22:04 UTC</p>
</html>